Closed Bug 1376158 Opened 8 years ago Closed 8 years ago

Stack exhaustion in [@ AppendText]

Categories

(Core :: MathML, defect)

55 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox-esr52 --- unaffected
firefox54 --- unaffected
firefox55 --- fixed
firefox56 --- fixed

People

(Reporter: tsmith, Assigned: emilio)

References

(Blocks 1 open bug)

Details

(Keywords: crash, testcase)

Attachments

(2 files)

Attached file test_case.html
#0 0x00007fffdcaf92d5 in AppendText () at src/dom/base/nsLineBreaker.cpp:330 #1 0x00007fffe0d4737b in SetupBreakSinksForTextRun () at src/layout/generic/nsTextFrame.cpp:2647 #2 0x00007fffe0d40874 in BuildTextRunForFrames () at src/layout/generic/nsTextFrame.cpp:2408 #3 0x00007fffe0d3a8f7 in FlushFrames () at src/layout/generic/nsTextFrame.cpp:1688 #4 0x00007fffe0d4ae4f in BuildTextRuns () at src/layout/generic/nsTextFrame.cpp:1614 #5 EnsureTextRun () at src/layout/generic/nsTextFrame.cpp:2861 #6 0x00007fffe0d82736 in AddInlineMinISizeForFlow () at src/layout/generic/nsTextFrame.cpp:8455 #7 0x00007fffe0d85464 in AddInlineMinISize () at src/layout/generic/nsTextFrame.cpp:8625 #8 0x00007fffe0b090cf in GetMinISize () at src/layout/generic/nsBlockFrame.cpp:770 #9 0x00007fffe0b7b356 in ShrinkWidthToFit () at src/layout/generic/nsFrame.cpp:5800 #10 ComputeAutoSize () at src/layout/generic/nsContainerFrame.cpp:845 #11 0x00007fffe0b81f97 in ComputeSize () at src/layout/generic/nsFrame.cpp:5061 #12 0x00007fffe0ac3b61 in InitConstraints () at src/layout/generic/ReflowInput.cpp:2492 #13 0x00007fffe0aba110 in Init () at src/layout/generic/ReflowInput.cpp:409 #14 0x00007fffe10e7cce in Reflow () at src/layout/mathml/nsMathMLTokenFrame.cpp:143 #15 0x00007fffe0af58a4 in ReflowChild () at src/layout/generic/nsContainerFrame.cpp:978 #16 0x00007fffe10d8d96 in ReflowChild () at src/layout/mathml/nsMathMLContainerFrame.cpp:839 #17 0x00007fffe10d96cd in Reflow () at src/layout/mathml/nsMathMLContainerFrame.cpp:892 #18 0x00007fffe0af58a4 in ReflowChild () at src/layout/generic/nsContainerFrame.cpp:978 #19 0x00007fffe10d8d96 in ReflowChild () at src/layout/mathml/nsMathMLContainerFrame.cpp:839 #20 0x00007fffe10d96cd in Reflow () at src/layout/mathml/nsMathMLContainerFrame.cpp:892 #21 0x00007fffe0cc71df in ReflowFrame () at src/layout/generic/nsLineLayout.cpp:921 #22 0x00007fffe0cc5153 in ReflowInlineFrame () at src/layout/generic/nsInlineFrame.cpp:798 #23 0x00007fffe0cc3597 in ReflowFrames () at src/layout/generic/nsInlineFrame.cpp:681 #24 0x00007fffe0cc26ba in Reflow () at src/layout/generic/nsInlineFrame.cpp:460
Flags: in-testsuite?
INFO: Last good revision: aab0dfdae32f14246e3bed8824a5f7ce309276cd INFO: First bad revision: 3e6477d932037d6026ac13bd8c988dc0a51935d4 INFO: Pushlog: https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=aab0dfdae32f14246e3bed8824a5f7ce309276cd&tochange=3e6477d932037d6026ac13bd8c988dc0a51935d4
Blocks: 1361766
Flags: needinfo?(emilio+bugs)
Version: Trunk → 55 Branch
I don't know a lot about MathML, but I don't think that should be valid markup... Pretty much every change to the test case makes it invalid markup... Here's a test-case with the balanced tags: <math> <munderover> &gt; <munder accentunder="true"> <mo>)</mo> <mscarry> </munder> </munderover> </math> I _think_ (reading https://www.w3.org/TR/MathML3/chapter3.html#presm.mscarries) that <mscarry> should only be allowed inside <mscarries>, but I'm not totally sure... Loading this test-case in a debug build I get a bunch of: WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x80004001: file /home/emilio/projects/moz/stylo/layout/generic/nsBlockFrame.cpp, line 946...
That being said I think I know why the test-case regressed with my patch, which is kind of sad... MathML reflows are non-idempotent, which is something someone should really fix :(
Comment on attachment 8881163 [details] Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content. https://reviewboard.mozilla.org/r/152432/#review158550
Attachment #8881163 - Flags: review?(xidorn+moz) → review+
Pushed by ecoal95@gmail.com: https://hg.mozilla.org/integration/autoland/rev/5280f9ba46df Don't let non-primary frames affect the mIncrementScriptLevel of the content. r=xidorn
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Should we uplift this change to beta?
Assignee: nobody → emilio+bugs
Flags: needinfo?(emilio+bugs)
(In reply to Julien Cristau [:jcristau] from comment #9) > Should we uplift this change to beta? Seems harmless, yeah, will file a request.
Flags: needinfo?(emilio+bugs)
Comment on attachment 8881163 [details] Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content. Approval Request Comment [Feature/Bug causing the regression]: bug 1361766 [User impact if declined]: Crashes on some obscure MathML content. [Is this code covered by automated tests?]: Yes [Has the fix been verified in Nightly?]: Yes [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: Just this patch. [Is the change risky?]: No. [Why is the change risky/not risky?]: This just enforces an invariant that is assumed in the rest of the MathML code, and only affects already-invalid MathML content. [String changes made/needed]: none
Attachment #8881163 - Flags: approval-mozilla-beta?
Comment on attachment 8881163 [details] Bug 1376158: Don't let non-primary frames affect the mIncrementScriptLevel of the content. avoid stack exhaustion, regression in beta55
Attachment #8881163 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Emilio Cobos Álvarez [:emilio] from comment #11) > [Is this code covered by automated tests?]: Yes > [Has the fix been verified in Nightly?]: Yes > [Needs manual test from QE? If yes, steps to reproduce]: No Setting qe-verify- based on Emilio's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: